fix(sandboxes): harden live-process/exec RPC surface against transient link faults - #837
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df6d8250e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df6d8250e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…sient link faults The client<->sandbox Connect-RPC surface had three separate ways a transient network blip became a fatal error and killed a rollout: 1. background-job launch (`start_background_job`) — a 30s exec with no retry; 2. live-process control RPCs (`_execute_process_control_rpc`) — only retried UNAUTHENTICATED; 3. the live-process output stream — a mid-stream read fault (`process stream RPC failed (unavailable): ... error reading a body from connection: timed out`) tore down the still-running process, the largest killer of long agentic rollouts. Consolidate the ad-hoc handling into one reliability policy (`_reliability.py`): - `is_transient_rpc_error()` — one classifier (DEADLINE_EXCEEDED / UNAVAILABLE / ABORTED / body-read timeout / connection reset = transient; 404 sandbox-not-found and other 4xx = permanent), used everywhere. - launch + control RPCs retry transient faults with bounded exponential backoff (folds in the earlier standalone launch-retry). - the output stream RE-ATTACHES to the still-running process via the `Connect` server-streaming RPC (by pid) and resumes, instead of failing the rollout; bounded reconnects with backoff. - live-process transport keeps the long-lived stream connection warm (TCP keepalive + generous pool-idle timeout) so a brief stall does not tear it down in the first place. - all timeouts / retry budgets are env-configurable (`PRIME_SANDBOX_*`), no magic 30s. Adds hermetic unit tests: transient classification, stream reconnect-and-resume (incl. no-reconnect and permanent-fault paths), and launch retry (sync + async). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… stream fault A second production variant of the broken-output-stream fault surfaces as gRPC INTERNAL "Error reading content" (vs the UNAVAILABLE "... timed out" variant already handled). It is the same transient stream-break class, so add Code.INTERNAL to the transient codes and "reading content" to the message markers; the output stream now reconnects on it too. Confirmed in production: HarnessErrors dropped from ~21/window to 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
6a21fa0 to
3ef1816
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3ef1816. Configure here.

Transient client↔VM link failures no longer kill a rollout when the process is still running. Live output streams reattach through
CommandSession.Connectwith bounded backoff, including clean EOF. If the initial PID event is lost, an SDK-generated session tag is resolved throughListbefore attaching by PID. Reattachment tails from the current point, so output emitted during the gap is not replayed.Background-job launch retries remain bounded and use an atomic
mkdirguard, preventing duplicate execution after an ambiguous timeout. This subsumes #832.Stdin and signal control RPCs deliberately do not retry transient failures: the server may have already applied the operation. Rejected auth is refreshed once. The fuzzy error classifier and speculative environment configuration have been removed in favor of local constants and direct reconciliation.
Follow-up work will add proper idempotency semantics in
sandboxdforStart,SendInput, andSendSignalbefore enabling live-process start/control retries.Note
Medium Risk
Changes long-lived live-process streaming, session discovery, and background job launch semantics; behavior is bounded and heavily tested but affects rollout-critical VM exec paths.
Overview
Transient link failures no longer tear down VM live processes while the remote command is still running.
AsyncSandboxProcesscan re-attach the output stream (up to five times with exponential backoff) when the RPC stream errors or ends without an exit event; reattachment uses a new optionalreconnecthook and does not replay output from the disconnect window.open_processnow passes that hook: each start gets a uniqueprime-sdk-*session tag, wiresCommandSession.Connectfor reattach by PID, and usesListto resolve the PID when the initial start event was lost. Live-process HTTP transport adds TCP keepalive and a longer pool idle timeout.Background job launch (sync and async) retries on ambiguous
CommandTimeoutErrorwith bounded backoff and an atomicmkdirlaunch directory so a timeout retry cannot spawn duplicate jobs.RPC helpers add Connect/List method metadata and request builders plus optional tag on start requests. Stdin/signal control RPCs are unchanged (no transient retries). New tests cover stream reconnect, tag discovery with auth refresh, and launch retries.
Reviewed by Cursor Bugbot for commit ffcc86e. Bugbot is set up for automated code reviews on this repo. Configure here.